Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Escaping for Identifiers and Literals in Migrations #538

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Add Escaping for Identifiers and Literals in Migrations #538

merged 2 commits into from
Nov 2, 2023

Conversation

grahamhoyes
Copy link
Contributor

@grahamhoyes grahamhoyes commented Oct 27, 2023 •
edited
Loading

Fixes #537

Adds escaping for identifiers (table names, users, etc) using pq.QuoteIdentifier, and for literals using pq.QuoteLiteral. This allows usernames with characters like - and @, passwords with ', and resolves a SQL injection vulnerability.

Updates github.com/lib/pq to the latest version. It needed to be upgraded to at least v1.2.0, which added the QuoteLiteral function.

Verification done thus far:

  • Ran homer image targeting a database with a regular username/password
  • Ran homer targeting a GCP Cloud SQL database with IAM authentication (which has - and @ in the username)
  • Ran the initialization commands in the readme
  • Loaded the image onto a dev environment with a real setup and poked around in it

@CLAassistant
Copy link

CLAassistant commented Oct 27, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@grahamhoyes grahamhoyes marked this pull request as ready for review October 27, 2023 18:20
@grahamhoyes grahamhoyes changed the title Add Escaping for Identifiers and Literals in Migrations (Fix #537) Add Escaping for Identifiers and Literals in Migrations Oct 27, 2023
@grahamhoyes
Copy link
Contributor Author

Got this running in a few environments, everything seems to be working as expected. @lmangani if there's any other testing you'd like to see let me know, otherwise this should be good for you to review.

@lmangani lmangani added the enhancement New feature or request label Oct 31, 2023
@lmangani
Copy link
Member

Thanks @grahamhoyes the change is under review and we'll have an update soon! We appreciate your contribution!

@lmangani
Copy link
Member

lmangani commented Nov 2, 2023

@adubovikov please review whenever possible 👍

@adubovikov adubovikov merged commit 8361b3b into sipcapture:master Nov 2, 2023
3 checks passed
@grahamhoyes
Copy link
Contributor Author

Apologies if there's a proper process I'm missing, but would it be possible to make a release that contains this @adubovikov? 1.4.59 was made just before this was merged.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adubovikov adubovikov adubovikov approved these changes

Assignees

@adubovikov adubovikov

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SQL identifiers and string literals aren't escaped in migration.go
4 participants
@grahamhoyes @CLAassistant @lmangani @adubovikov